Fix #10944: unused EMS actuator warning never fires by brianlball · Pull Request #11525 · NatLabRockies/EnergyPlus

brianlball · 2026-04-14T15:09:40Z

Summary

Fixes Provide a warning for unused actuators #10944 - end-of-sim warning for unused EMS actuators never fired.
Root cause: checkForUnusedActuatorsAtEnd checked ErlVariable.Value.initialized, but that flag has dual meaning: "safe to read as right-hand-side operand" AND "was SET by Erl program". Actuators get initialized=true early (inherited from the Null sentinel at registration) so the first meaning applies cleanly - but the guard read it as the second meaning and never tripped.
Fix: add a dedicated wasActuated bool on ActuatorUsedType. Set it only when the user's Erl program assigns a concrete value to the actuator (Type != Null in the post-ManageEMS actuator-update loop at EMSManager.cc:345). Check that flag in the end-of-sim guard. Leaves Value.initialized and the Null sentinel alone.

Why this approach (history)

First attempt flipped the Null sentinel's initialized flag to false. That seemed surgical but broke right-hand-side reads of actuators that hadn't been SET yet but had subsystem-provided defaults - the expression evaluator at RuntimeLanguageProcessor.cc:1800 reads the same flag to gate "is this variable safe to use in an expression." CI caught it via regressions in integration.EMSUserDefined5ZoneAirCooled and integration.PythonPluginUserDefined5ZoneAirCooled. The current fix matches Myoldmopar's original suggestion in the issue ("a simple flag on each actuator").

Call graph - issue & fix

BEFORE FIX
==========
  IDF: EMS:Actuator A1  +  EMS:Program "Set Al = 2" (typo, A1 untouched)
         |
  [EMSManager.cc:1988]  if (!ErlVariable[A1].Value.initialized)
                                                  ^^^^^^^^^^^
                        true (inherited from Null sentinel at setup)
                        so !true = false -> guard never passes
         v
  (no warning emitted)  X


AFTER FIX
=========
  ActuatorUsedType gains bool wasActuated = false  (default)

  [EMSManager.cc:345]  // post-ManageEMS actuator-update loop
    for each EMSActuatorUsed:
        if (ErlVar.Value.Type != Null) {                // user SET a concrete value
            thisActuatorUsed.wasActuated = true;        // <- NEW
            ...push value to component, set *Actuated=true...
        }

  [EMSManager.cc:1988]  if (!EMSActuatorUsed[i].wasActuated)     // <- NEW guard
         v
  ShowWarningError: "Unused EMS Actuator detected ... A1"  OK

  Null sentinel semantics: untouched.
  Value.initialized semantics (right-hand-side readability): untouched.

Files changed

src/EnergyPlus/DataRuntimeLanguage.hh - add wasActuated field on ActuatorUsedType
src/EnergyPlus/EMSManager.cc - set the flag in the actuator-update else branch; check it in the end-of-sim guard
tst/EnergyPlus/unit/EMSManager.unit.cc - new UnusedActuatorWarning test

Test plan

New EnergyPlusFixture.UnusedActuatorWarning asserts warning fires for unused actuator
Full unit test suite post-fix: 2213 tests passed, 0 failures
integration.EMSUserDefined5ZoneAirCooled: passes locally (was the CI regression in the prior attempt)
All 18 integration.EMS* tests: pass
pre-commit (clang-format v19, constexpr, license): clean
scripts/check_gcc_warnings.py src/EnergyPlus/EMSManager.cc: clean

IDF with actuator A1 + program `Set Al = 2` (typo). Expect err stream contains "Unused EMS Actuator detected A1" after checkForUnusedActuatorsAtEnd; stream is empty on develop. Fails on current source - verifies the gap.

Prior attempt flipped Null sentinel initialized=false, which broke right-hand-side reads of actuators that hadn't been SET yet but had subsystem-provided default values (regressed EMSUserDefined5ZoneAirCooled, PythonPluginUserDefined5ZoneAirCooled). Value.initialized has dual meaning - both "safe to read as right-hand-side operand" and "was SET by Erl" - so co-opting it for the warning check was wrong. Per Myoldmopar's suggestion in the issue, add a dedicated `wasActuated` bool on ActuatorUsedType. Set it in the EMSManager.cc post-ManageEMS actuator-update loop (else branch where Type != Null, i.e. user SET a concrete value). Check it in checkForUnusedActuatorsAtEnd instead of Value.initialized. Leaves Null sentinel and Value.initialized semantics untouched.

github-actions · 2026-04-14T16:43:20Z

⚠️ Regressions detected on ubuntu-24.04 for commit `87694ef`

Regression Summary

ERR: 13

brianlball · 2026-04-14T16:52:19Z

brianlball · 2026-04-14T16:59:37Z

Regression diffs — all expected, warning working as intended

The regression tool flagged 13 files with diffs. All 13 are ERR-file-only — no ESO, no tabular, no SQL. Simulation output values are identical to develop baseline across all 13. The diffs are purely the new Unused EMS Actuator detected warning firing on actuators declared in these IDFs that no Erl program ever SETs — exactly what #10944 is asking this warning to catch.

Pattern breakdown

Cluster	Unused actuator names	Files (count × warns)	Nature
Apartment/hotel setpoint templates	`S_APARTMENT_CLGSETP_ACTUATOR`, `S_APARTMENT_HTGSETP_ACTUATOR`, `S_GUESTROOM_CLGSETP_ACTUATOR`, `S_GUESTROOM_HTGSETP_ACTUATOR`	ApartmentHighRise ×4, ApartmentMidRise ×4, HotelSmall ×4	Prototype-building boilerplate: `Schedule:Compact` setpoint actuators declared for a family of spaces, only a subset driven
Hospital extra water heat SPs	`PATRMS_EXTRAWATERHEATC_SP`, `ER_EXTRAWATERHEATC_SP` (+3 more)	Hospital ×5	`System Node Setpoint` actuators declared but unused
OfficeLarge chiller pump flow	`CHILLER1PUMPFLOW`	OfficeLarge ×1, _Chiller205 ×1, _Chiller205_Detailed ×1, _ChillerEIR ×1	Shared root IDF; 4 chiller-variant tests inherit one orphan
Restaurant lights	`DINING_LIGHT_ACTUATOR`, `KITCHEN_LIGHT_ACTUATOR`	RestaurantFastFood ×2, RestaurantSitDown ×2	`Lights` actuators declared as scaffolding
EMS demand/manager demos	`HEATSYS1_BOILER_SETPOINT`	EMSDemandManager_LargeOffice ×1, EMSReplaceTraditionalManagers_LargeOffice ×1	Demo IDF with a declared-but-never-SET boiler setpoint actuator
Python plugin variant	(not run locally)	PythonPluginUserDefinedWindACAuto	Almost certainly same flavor — IDF declares actuator, Python `setActuatorValue` takes over bypassing Erl

Sample hit (EMSDemandManager_LargeOffice)

** Warning ** checkForUnusedActuatorsAtEnd: Unused EMS Actuator detected...
**   ~~~   ** Check Erl programs related to EMS actuator variable name = HEATSYS1_BOILER_SETPOINT
**   ~~~   ** EMS Actuator type name = SYSTEM NODE SETPOINT
**   ~~~   ** EMS Actuator unique component name = HEATSYS1 SUPPLY EQUIPMENT OUTLET NODE
**   ~~~   ** EMS Actuator control type = TEMPERATURE SETPOINT

Conclusion

Keeping the warnings as-is. Each flagged actuator is a genuine declared-but-unused case. If the corpus authors meant these as placeholders, the warning now documents that; if they're dead, it flags them for cleanup. Either way, this is the fix working on real-world data.

github-actions · 2026-04-14T17:34:36Z

⚠️ Regressions detected on macos-14 for commit `87694ef`

Regression Summary

ERR: 13

Myoldmopar

This seems like a reasonable change to get a nice warning if an ERL actuator is not used. My only additional comment would be that it would be nice to clean out the warnings that are now added from unused actuators in the test files. Even when there is good intention to clean out warnings later, it's very easy to not do them. Otherwise this is good.

Myoldmopar · 2026-04-15T16:58:46Z

        bool CheckedOkay;              // set to true once matched to available actuator
        int ErlVariableNum;            // points to global Erl variable, matches Name
        int ActuatorVariableNum;       // points to index match in EMSActuatorAvailable structure
+        bool wasActuated = false;      // issue #10944: true once any Erl program has set this actuator


Adding a flag here seems quite reasonable.

Myoldmopar · 2026-04-15T16:59:40Z

+    EMSManager::ManageEMS(*state, EMSManager::EMSCallFrom::SetupSimulation, anyRan, ObjexxFCL::Optional_int_const());
+    EMSManager::ManageEMS(*state, EMSManager::EMSCallFrom::BeginTimestepBeforePredictor, anyRan, ObjexxFCL::Optional_int_const());
+
+    compare_err_stream(""); // drop any setup chatter


Good unit test. I'm not sure this call to compare_err_stream is needed since you are properly using the compare_err_stream_substring function below. The err stream does not need to be cleared of any chatter. It's not harming anything, but not necessary in the future.

Runtime wasActuated flip false-positived on idiomatic IF cond SET=v ELSE SET=NULL pattern when design-day runs never hit non-null branch. Move check to parse-time: flip on Erl SET target match. Python path flips on getActuatorHandle match. External/FMU already outside warning loop range.

BypassHXStatus + Chiller1PumpFlow from Hospital; Chiller1PumpFlow from OfficeLarge + 3 Chiller variants; HeatSys1_Boiler_Setpoint from two EMS demo LargeOffice IDFs. All declared, none ever SET in any Erl program.

…ears Python-API test in EMSManager.unit.cc violated testing pattern and failed Linux CI (missing include + state.get() on raw pointer). Keep single Python test in datatransfer.unit.cc where fixture belongs. Also drop compare_err_stream("") pre-clears per review - substring search is targeted, collision with setup chatter impossible.

github-actions · 2026-04-15T22:12:04Z

⚠️ Regressions detected on ubuntu-24.04 for commit `dca0057`

Regression Summary

Audit: 7

github-actions · 2026-04-15T22:31:15Z

⚠️ Regressions detected on macos-14 for commit `dca0057`

Regression Summary

Audit: 7

brianlball · 2026-04-15T22:49:58Z

Trying to clean up the 13 flagged IDFs surfaced a problem with the runtime wasActuated flip.

Of the 13, only 2 actuator declarations were truly dead:

CHILLER1PUMPFLOW — OfficeLarge + 3 Chiller variants (and Hospital, caught during verification)
HeatSys1_Boiler_Setpoint — EMSDemandManager + EMSReplaceTraditionalManagers
plus BypassHXStatus in Hospital (program named for it, never actually SET)

The other 11 hits are the idiomatic EMS override pattern:

IF trigger, SET act = override, ELSE SET act = NULL, ENDIF;

SET = NULL is the documented way to release an actuator. When the design-day run never hits the trigger branch, the runtime check at EMSManager.cc:343-346 sees only null values, wasActuated stays false, and the warning falsely fires on fully-wired-up EMS.

Re-reading #10944 — the goal is typo detection: SET Al = 2 when the actuator is A1 creates a fresh local Al; A1 is never a SET target anywhere. That is a static property, not a runtime one. A parse-time scan for SET-target matches catches every typo case while silencing the NULL-branch false positives.

Changes:

RuntimeLanguageProcessor.cc:458 — on every SET instruction emission, if LHS resolves to an actuator's ErlVariable, flip wasActuated. O(actuators) per SET, once at parse, zero per-timestep cost.
datatransfer.cc:~430 — Python getActuatorHandle flips wasActuated when the handle matches an IDF-declared actuator. Handle retrieval is the usage signal for Python (Python catches typos itself — getActuatorHandle returns -1). Piggybacks on the existing duplicate-check loop.
EMSManager.cc:346 — dropped runtime flip; static scan subsumes it.
EMSManager.unit.cc — new test UnusedActuatorWarning_ConditionalNullBranchNotFalsePositive pins the false-positive fix. Existing typo test still passes.
datatransfer.unit.cc — new test DataTransfer_PythonHandle_MarksActuatorAsUsed for the Python path.
Removed the 3 truly-dead actuator sets from the 7 flagged IDFs.

No `Unused EMS Actuator` warnings anywhere in the integration corpus. Regression tool on this branch: previous ERR: 13 → now Audit: 7 (actuator-count bookkeeping only, no simulation-output diffs).

Pattern for parse-time flag flips mirrors #11409 (`SetByGlobalVariable` / `SetByInternalVariable`) — same style, reviewer-blessed precedent.

repro #10944: unused actuator warning never fires

7c0eee9

IDF with actuator A1 + program `Set Al = 2` (typo). Expect err stream contains "Unused EMS Actuator detected A1" after checkForUnusedActuatorsAtEnd; stream is empty on develop. Fails on current source - verifies the gap.

brianlball added the Defect Includes code to repair a defect in EnergyPlus label Apr 14, 2026

brianlball force-pushed the unused_actuator_warn branch from 1179f5f to 2469735 Compare April 14, 2026 15:22

brianlball added the DoNotMerge Code that requires additional attention and investigation label Apr 14, 2026

brianlball force-pushed the unused_actuator_warn branch from 2469735 to e91fc19 Compare April 14, 2026 16:17

NatLabRockies deleted a comment from github-actions bot Apr 14, 2026

brianlball removed the DoNotMerge Code that requires additional attention and investigation label Apr 14, 2026

brianlball requested review from Myoldmopar April 14, 2026 17:42

Myoldmopar approved these changes Apr 15, 2026

View reviewed changes

brianlball added 3 commits April 15, 2026 16:39

#10944 remove truly-unused actuators from test IDFs

cc9bca0

BypassHXStatus + Chiller1PumpFlow from Hospital; Chiller1PumpFlow from OfficeLarge + 3 Chiller variants; HeatSys1_Boiler_Setpoint from two EMS demo LargeOffice IDFs. All declared, none ever SET in any Erl program.

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix #10944: unused EMS actuator warning never fires#11525

Fix #10944: unused EMS actuator warning never fires#11525
brianlball wants to merge 5 commits intodevelopfrom
unused_actuator_warn

brianlball commented Apr 14, 2026 •

edited

Loading

Uh oh!

github-actions bot commented Apr 14, 2026

Uh oh!

brianlball commented Apr 14, 2026

Uh oh!

brianlball commented Apr 14, 2026 •

edited

Loading

Uh oh!

github-actions bot commented Apr 14, 2026

Uh oh!

Myoldmopar left a comment

Uh oh!

Myoldmopar Apr 15, 2026

Uh oh!

Myoldmopar Apr 15, 2026

Uh oh!

github-actions bot commented Apr 15, 2026

Uh oh!

github-actions bot commented Apr 15, 2026

Uh oh!

brianlball commented Apr 15, 2026 •

edited

Loading

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

brianlball commented Apr 14, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Why this approach (history)

Call graph - issue & fix

Files changed

Test plan

Uh oh!

github-actions bot commented Apr 14, 2026

⚠️ Regressions detected on ubuntu-24.04 for commit 87694ef

Uh oh!

brianlball commented Apr 14, 2026

Uh oh!

brianlball commented Apr 14, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Regression diffs — all expected, warning working as intended

Pattern breakdown

Sample hit (EMSDemandManager_LargeOffice)

Conclusion

Uh oh!

github-actions bot commented Apr 14, 2026

⚠️ Regressions detected on macos-14 for commit 87694ef

Uh oh!

Myoldmopar left a comment

Choose a reason for hiding this comment

Uh oh!

Myoldmopar Apr 15, 2026

Choose a reason for hiding this comment

Uh oh!

Myoldmopar Apr 15, 2026

Choose a reason for hiding this comment

Uh oh!

github-actions bot commented Apr 15, 2026

⚠️ Regressions detected on ubuntu-24.04 for commit dca0057

Uh oh!

github-actions bot commented Apr 15, 2026

⚠️ Regressions detected on macos-14 for commit dca0057

Uh oh!

brianlball commented Apr 15, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

brianlball commented Apr 14, 2026 •

edited

Loading

⚠️ Regressions detected on ubuntu-24.04 for commit `87694ef`

brianlball commented Apr 14, 2026 •

edited

Loading

⚠️ Regressions detected on macos-14 for commit `87694ef`

⚠️ Regressions detected on ubuntu-24.04 for commit `dca0057`

⚠️ Regressions detected on macos-14 for commit `dca0057`

brianlball commented Apr 15, 2026 •

edited

Loading